Skip to content

EIP4844: Remove reliance on py-ecc types in polynomial-commitments and add py-arkworks#3272

Closed
kevaundray wants to merge 18 commits intoethereum:devfrom
kevaundray:kw/py-ecc-agnostic-4844
Closed

EIP4844: Remove reliance on py-ecc types in polynomial-commitments and add py-arkworks#3272
kevaundray wants to merge 18 commits intoethereum:devfrom
kevaundray:kw/py-ecc-agnostic-4844

Conversation

@kevaundray
Copy link
Contributor

@kevaundray kevaundray commented Feb 24, 2023 •
edited
Loading

Description

Currently the polynomial-commitments file relies on py-ecc types and methods. This can be seen with the usage of bls.Z1 and bls.G1. If we do plan to use another library, removing this decoupling is a necessary pre-requisite.

Changes

  • Removed Z2 and FQ from imports in bls.py as they were not being used
  • aliased all py_ecc direct imports that we need for 4844 with a py_ecc prefix
  • Added wrapper methods which for now directly call py_ecc, but in the future can conditionally call a different library

Note

I have intentionally not tried to change method names, so the changes in polynomial-commitments are easy to grep.

EDIT:

  • Dankrad has also added in the py-arkworks library into this PR.

@kevaundray kevaundray force-pushed the kw/py-ecc-agnostic-4844 branch from cbeb7cf to f59dd7c Compare February 24, 2023 17:56
kevaundray
kevaundray commented Feb 24, 2023
View reviewed changes
kevaundray
kevaundray commented Feb 24, 2023
View reviewed changes
@dankrad dankrad force-pushed the kw/py-ecc-agnostic-4844 branch from df9167f to dbfb1ac Compare February 25, 2023 22:26
@dankrad dankrad mentioned this pull request Feb 25, 2023
Merged
6 tasks
@kevaundray kevaundray changed the title EIP4844: Remove reliance on py-ecc types in polynomial-commitments EIP4844: Remove reliance on py-ecc types in polynomial-commitments and add py-arkworks Feb 26, 2023
kevaundray
kevaundray commented Feb 26, 2023
View reviewed changes
kevaundray
kevaundray commented Feb 26, 2023
View reviewed changes
kevaundray
kevaundray commented Feb 26, 2023
View reviewed changes
kevaundray
kevaundray commented Feb 26, 2023
View reviewed changes
hwwhww
hwwhww reviewed Feb 27, 2023
View reviewed changes
haxxpop
haxxpop reviewed Feb 28, 2023
View reviewed changes
@hwwhww
Copy link
Contributor

hwwhww commented Feb 28, 2023

((Moving arkworks discussion to this PR))

Hey @kevaundray, I just realized my PyPy3.9 was built with 0.3.2 because 0.3.3 doesn't provide "Source Distributions" on PyPi. Could you upload the source distributions for 0.3.3?

image

Also, for completeness, it would be nice to upload macos Python3.9 distributions too. 🙏

@hwwhww hwwhww added the testing CI, actions, tests, testing infra label Feb 28, 2023
haxxpop
haxxpop reviewed Feb 28, 2023
View reviewed changes
haxxpop
haxxpop reviewed Feb 28, 2023
View reviewed changes
@kevaundray
Copy link
Contributor Author

kevaundray commented Feb 28, 2023

((Moving arkworks discussion to this PR))

Hey @kevaundray, I just realized my PyPy3.9 was built with 0.3.2 because 0.3.3 doesn't provide "Source Distributions" on PyPi. Could you upload the source distributions for 0.3.3?

image

Also, for completeness, it would be nice to upload macos Python3.9 distributions too. 🙏

@hwwhww I've added 3.8 to 3.11 along with source distributions in version 0.3.4, please let me know if anything else is missing :)

@kevaundray
Copy link
Contributor Author

kevaundray commented Mar 3, 2023

Closing as this was merged into #3274 which was merged into dev yesterday

@kevaundray kevaundray closed this Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hwwhww hwwhww hwwhww left review comments

+2 more reviewers

@haxxpop haxxpop haxxpop left review comments

@dankrad dankrad dankrad left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

testing CI, actions, tests, testing infra

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kevaundray @hwwhww @haxxpop @dankrad